fix(dry-run): thread { dryRun } through the loaders pull, push, status and list use - #866

• [P1 blocking] src/pull.ts:1885 — In a fresh self-mode clone without local config, dry-run detection returns an in-memory config whose partition does not exist. lockScope() then calls acquireLock(), which creates that partition directory; releasing the lock leaves the directory behind. Thus pull --dry-run still changes the filesystem.
• [P1 blocking] src/push.ts:731 — The same previewed self-mode config continues into the normal self-mode setup before pushCore reaches its dry-run guard. Besides creating the partition for .sync-lock, an older clone triggers migrateSelfModeGitignore() and rewrites .teamai/.gitignore; the command also creates a disposable worktree. push --dry-run therefore still performs changes.
• The PR description’s testing record is otherwise sufficient, including representative real-CLI verification, but its fixtures do not run pull/push against the existing fresh-self-clone case that exposes these paths.

• [P1 blocking] src/pull.ts:1870 — A dry run now pretends it holds the scope lock, but pullForScope() still calls publishQueuedLearnings(..., { holdsSyncLock: true }). With a queued learning, pull --dry-run can commit/push it and remove it from the local queue without acquiring either the skipped sync lock or honoring preview mode.
• [P1 blocking] src/pull.ts:1870 — Skipping the lock for every backend allows a git-mode pull --dry-run to call pullRepo() on the shared clone while another push holds the lock and may have a transient branch checked out. The preview can reset/read the writer’s checkout and interfere with the concurrent push; dry runs still need a non-writing lock acquisition strategy or must skip clone mutation.
• [P1 blocking] src/push.ts:788 — Passing the active self-mode checkout directly to pushCore() removes the clean worktree baseline that self-mode scanners rely on. For example, after editing .teamai/env/env.yaml, EnvHandler compares the file with the identical path because projectRoot/.teamai === repo.localPath, so push --dry-run reports no env change while a real push, which compares against the worktree, includes it.

The two findings from the earlier review are resolved: the project partition lock directory is no longer created, and self-mode push preview no longer runs the gitignore migration or disposable worktree setup. The PR description includes sufficient representative real-CLI verification.

• [P1 blocking] src/pull.ts:1871 — The dry-run lock check still lets pullForScope() call publishQueuedLearnings(..., { holdsSyncLock: true }). With an empty queue this creates ~/.teamai/locks/; with queued entries it can commit/push them and delete the local queue. pull --dry-run therefore still makes persistent changes.
• [P1 blocking] src/update.ts:395 — Returning the observed lock state is not equivalent to holding the lock. After a dry run sees “missing,” a real pull/push can acquire the lock while the preview continues into the same clone or fixed knowledge-wt path, allowing concurrent resets, fetches, or worktree removal. The dry-run path must avoid shared mutations or retain real exclusion without filesystem writes.
• [P1 blocking] src/pull.ts:1871 — A fresh self-mode config now reaches refreshTeamRepo(), which unconditionally calls migrateSelfModeGitignore(). If the clone has a pre-beta.5 .teamai/.gitignore, pull --dry-run rewrites that tracked file.
• [P1 blocking] src/push.ts:813 — Self-mode push --dry-run still enters withKnowledgeWorktree() and pushCore(). This always performs a fetch that persists .git/FETCH_HEAD, and when team resources differ, the pre-scan sync can update local tool files and state before the dry-run guard at src/push.ts:1577.
• The earlier partition-directory leak and self-mode baseline under-reporting are resolved. The PR description contains sufficient representative real-CLI testing.

addressed the four P1 findings from the latest review. All four are one mistake in different places: the preview loads a config that is structurally identical to a real one, and the code that then acts on what it loaded did not know it was looking at a preview.

Committed as df2d1bb on ydflow:fix/dry-run-preview-r5 (rebased onto your head a8325f6, nothing else changed).

src/pull.ts:1871 — the queue publish

Correct, and the fix also removes the ~/.teamai/locks/ entry the description declared as out of scope. publishQueuedLearnings now counts the queue through listPendingLearnings — a plain directory listing — instead of listPendingForInstall, which takes the queue lock so acquireLock creates its parent. Under a dry run it returns that count and publishes nothing: no commit, no push, no dropPendingLearning, no lock. That call was the only thing creating locks/, so the entry is gone rather than merely re-declared.

src/update.ts:395 — the lock answer is not the lock

Correct about the primitive, and the consequence was worse than the finding says: the preview did not merely claim a lock it did not hold, it went on to pull the shared clone. pullRepo fast-forwards the clone and, on divergence, reset --hards it — exactly the write the lock exists to exclude, performed by a process that holds nothing. A concurrent push that took the lock a moment after the preview called it free may have a transient branch checked out at that instant.

So the guard moved to where the mutation is: under a dry run refreshTeamRepo calls fetchTeamRepoReadOnly (git fetch origin <branch>) instead of pullRepo. Only the remote-tracking refs move, no working tree and no reset, so the preview still names the destination a real pull would sync from — which is what Revision 4 needed the worktree baseline for in the first place. acquireLock's read-only return stays as it is: it answers "would the real run have contended", which is a fact about the lock, not a claim about this process.

src/pull.ts:1871 — the gitignore self-heal

Correct. refreshTeamRepo's self branch called migrateSelfModeGitignore() unconditionally, rewriting a tracked file in the user's active tree, which outlives the preview. Now gated on options.dryRun, matching what push.ts:796 already did. It is idempotent, so the next real pull performs it.

src/push.ts:813 — the pre-scan state write

Half correct, and the half that is not worth changing is already settled. The git fetch that persists .git/FETCH_HEAD is declared in the description and stays: it is what lets the preview name the destination. The pre-scan syncTeamUpdatesToLocal also stays, and this is the part I would push back on — skipping it does not report an edit early, it hides it, which is #812's contract: outside the worktree, projectRoot/.teamai === repo.localPath in self mode, so the diff is empty by construction and every teammate update silently disappears from the preview. e2e/push-stale-worktree-812.test.ts:180 asserts the opposite of what removing it would give.

What was a genuine write is the saveStateForScope beside it. Recording "this checkout's copies were brought to rev X" when the real push will sync and record its own rev after its own sync is not part of answering what would be pushed — and it makes the next push compare against a revision this checkout never actually synced, which is the #812 failure in the other direction. Now skipped under options.dryRun.

Verification

oxlint --deny-warnings --report-unused-disable-directives   0 warnings, 0 errors (676 files)
tsc --noEmit                                                0 diagnostics

dry-run-load-path.test.ts    24/25   (the one failure is the fresh-clone
                                   `push --dry-run` case, which times out on
                                   `git fetch` of a remote that does not
                                   exist — it times out identically on your
                                   head `a8325f6`, verified by stashing these
                                   changes and re-running)
pull-scope-isolation.test.ts 14/14
lock-atomic.test.ts          pass

e2e/push-dry-run-writes-866.test.ts   1/1   new

The state-write case needs a project scope with a checkout record before recordsBase is true, which no fixture in dry-run-load-path.test.ts produces, so it lives in its own e2e file on the #812 fixture shape. Single-variable control, byte-identical test file:

unpatched head a8325f6    FAIL  (state.json rewritten)
df2d1bb                   PASS  (state.json byte-identical)

Two things I did not change, so you can decide rather than discover them:

• requireInitForScope (config.ts:625) still takes no options and calls loadLocalConfigForScope bare. It has no caller in src/, so it reads as exported surface rather than a live path — I left it alone rather than guess at its contract, as the description says.
• publishQueuedLearnings' busy branch previously recounted the queue without the lock (it could not hold the lock to count it). It now reuses the count taken before the lock was attempted, which is the same number one call later. Behaviourally identical, one fewer listing.

Correction to my comment above — it opens with a missing word (" addressed" should read "Addressed"). The content is otherwise as posted.

Also worth stating plainly, since it is the part a reader has to decide on: I pushed to ydflow:fix/dry-run-preview-r5 rather than to your fork, because the head repo of this PR is yours. To fold this into #866 the branch has to move, so either:

• git fetch https://github.com/ydflow/teamai-cli.git fix/dry-run-preview-r5 && git push origin HEAD:refs/heads/<your-head-branch>, or
• open a follow-up PR from ydflow:fix/dry-run-preview-r5 into Smilewithoutfalling's branch.

The commit is a single change on top of your head a8325f6 with nothing else touched, so it applies cleanly either way.

@ydflow — df2d1bb is correct, and I verified it independently before deciding what to do with it. Starting with the part you could not have known:

#866 was squash-merged 91 minutes after your comment, without df2d1bb

13:00:55Z  your comment (df2d1bb)
14:31:56Z  review   jeff-r2026  APPROVED
14:32:16Z  merge    jeff-r2026  → b3b3a0b03c
14:32:17Z  close

b3b3a0b03c has one parent, so the head commits were flattened into it. Your commit's parent, a8325f6, is not in main's history — and the PR is closed, so there is no branch left to fold into. A new PR against main is the only route now.

I measured your commit; it does what it says

Pulled the 5 files and checked them against the commit's blob shas — all match, crlf=0. Then drove the built CLI against real git fixtures and hashed the whole tree before and after:

14/14 assertions, including the two that decided Rev2/Rev3. This is strictly better than what I have: Rev5 gates the .gitignore rewrite, but locks/ and state.json still get written in my tree. The state.json write was sitting in my driver's output and I never attributed it; you found it.

None of it is in main

Second column above. The lock is created at learnings-publish.ts:93 — listPendingForInstall takes the queue lock, and that call sits above the dryRun early-return at line 126. Your finding, still live on main.

Rebase, not merge — conflict map

git's own merge engine, base = a8325f6, ours = b3b3a0b03c, theirs = df2d1bb, with an ancestry self-check so nothing could fast-forward:

src/pull.ts — main fixed the same line independently:

// current main
const queue = await publishQueuedLearnings(localConfig, localConfig.username, { holdsSyncLock: true, dryRun: options.dryRun });
if (options.dryRun) {
  if (queue.remaining > 0) log.info(`[${scopeLabel}] [dry-run] Would publish ${queue.remaining} queued learning(s)`);
} else if (queue.published.length > 0) {

// df2d1bb
const queue = await publishQueuedLearnings(localConfig, localConfig.username, {
  holdsSyncLock: true,
  ...(options.dryRun ? { dryRun: true } : {}),
});
if (queue.published.length > 0) {

Take main's side. Your conditional spread was correct against the branch's signature ({ holdsSyncLock?: boolean }); main's already accepts dryRun, and its Would publish N line comes for free.

src/utils/learnings-publish.ts is the real work. Main grew it from 211 to 556 lines, splitting the lock out of the body into publishQueuedLearnings + publishUnderSyncLock, so your patch is written against a function boundary that has moved. Three notes that may save a round:

• listPendingLearnings is not a drop-in: listPendingForInstall returns listed | busy | changed, and the switch (listing.status) below it depends on that union. listPendingLearnings returns string[].
• In main's shape the narrow fix is an early return above line 93 — if (dryRun) return { published: [], remaining: (await listPendingLearnings(localConfig)).length };. listPendingLearnings is already imported and already used that way at line 100, and this keeps pull.ts's Would publish N. It gives up busy/changed reporting under a preview, which a preview does not contend for anyway.
• Your dryRun?: true on PublishQueueReport does not conflict (main's type ends at installChanged), but main has no such field and does not need one — with the count in remaining, the caller reports it.

One more, mine, not yours

main:src/push.ts:805 carries a comment I wrote claiming env.ts compares projectRoot/.teamai against repo.localPath. There is no src/env.ts in this repo — I cited a file that does not exist, and it is in main now. That one is on me; it belongs in the follow-up alongside this.

My proposal

Open a new PR against main with your branch rebased. The four fixes are yours and should land under your name. Two things are already measured for that rebase, so it is not a blind one: take main's side of the single pull.ts conflict, and treat learnings-publish.ts as the one real piece of work (main split the function your patch is written against).

If you would rather not spend that rewrite on a file you did not break, say so and I will take learnings-publish.ts over and open the PR myself, with you as co-author. The other four files in your commit apply unchanged.

Either way I will verify the landed result the same way I verified df2d1bb — blob-level byte check plus the tree-hash driver — and report what I find.